Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQIL and PC performance check fixes #811

Merged
merged 5 commits into from
Oct 22, 2023
Merged

Conversation

ernestum
Copy link
Collaborator

@ernestum ernestum commented Oct 9, 2023

Description

  • Speeds up SQIL tests by decreasing the number of training steps
  • Skips SQIL performance tests because they are flaky
  • Adds some more seeds to SQIL tests
  • Fixes failing preference comparison performance test by increasing number of training steps and the number of samples to compare trained and novice agent.

Fixes #807
Fixes #791

Testing

Changes are covered by the existing tests.

@ernestum ernestum force-pushed the squil_performance_check_fixes branch from b1b1f13 to 1b20336 Compare October 11, 2023 08:23
@ernestum ernestum changed the title Squil performance check fixes SQIL and PC performance check fixes Oct 11, 2023
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #811 (1b20336) into master (20366b0) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #811      +/-   ##
==========================================
- Coverage   95.67%   95.63%   -0.05%     
==========================================
  Files         100      100              
  Lines        9581     9582       +1     
==========================================
- Hits         9167     9164       -3     
- Misses        414      418       +4     
Files Coverage Δ
tests/algorithms/test_preference_comparisons.py 100.00% <100.00%> (ø)
tests/algorithms/test_sqil.py 93.67% <100.00%> (-6.33%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ernestum
Copy link
Collaborator Author

Hi @AdamGleave would you review and merge? The coverage went down since I disabled the SQIL performance tests for now.

Copy link
Member

@AdamGleave AdamGleave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Sorry for the slow review. If I'm blocking something feel free to either ping me or get someone else to review (can start asking @dan-pandori for example).

@AdamGleave AdamGleave merged commit d833d9e into master Oct 22, 2023
14 of 15 checks passed
@AdamGleave AdamGleave deleted the squil_performance_check_fixes branch October 22, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix flaky SQIL test tests/algorithms/test_sqil.py::test_sqil_performance_continuous[DDPG] failure
2 participants